Skip to content

fix(openvpn): survive server-initiated soft reset and rekey - #3109

Open
ayanami-desu wants to merge 4 commits into
MetaCubeX:Alphafrom
ayanami-desu:fix/openvpn-rekey
Open

fix(openvpn): survive server-initiated soft reset and rekey#3109
ayanami-desu wants to merge 4 commits into
MetaCubeX:Alphafrom
ayanami-desu:fix/openvpn-rekey

Conversation

@ayanami-desu

Copy link
Copy Markdown

Fixes #3085.

Supersedes the closed review iteration in #3107. This version is rebased onto the current Alpha branch and incorporates the complete follow-up audit.

Summary

  • Treat P_CONTROL_SOFT_RESET_V1 as a fresh TLS/key epoch and follow OpenVPN's 0 → 1 → … → 7 → 1 key-ID sequence.
  • Preserve reliable control ordering, ACK state, replay protection, retransmission, session identity, and retiring data epochs across rekeys.
  • Make UDP control writes resilient to packet loss and temporary socket failures while surfacing permanent retransmission failures.
  • Anchor transition and outbound-promotion deadlines to soft-reset acceptance instead of delayed TLS/KM2 completion.
  • Preserve AUTH_PENDING, token refresh, push continuation, and shortened KM2 records across arbitrary TLS read boundaries.
  • Distinguish omitted tran-window from explicit zero; reject negative and overflowing values.
  • Fail closed before control/data packet-ID rollover and prevent AEAD nonce reuse.
  • Make control/TCP I/O cancellation-safe: deadlines and Close interrupt in-flight operations without dropping partial TCP frames or emitting queued payloads afterward.
  • Split TLS ciphertext into OpenVPN-compatible control datagrams and serialize complete TCP frames across partial writes.

Compatibility and safety details

  • tls-auth/tls-crypt packet IDs and replay state remain session-wide across soft resets.
  • Protected control timestamps remain stable until packet-ID rollover, matching OpenVPN 2.6 long-form packet IDs.
  • The previous data key remains available only for its configured transition window; expired writes pause until the replacement epoch is installed.
  • AUTH_FAILED, RESTART, HALT, and EXIT take precedence over coalesced push data without leaking adjacent auth tokens.
  • Malformed auth-token-user, invalid push-continuation, oversized control buffers, stale sessions, and invalid ACK arrays are rejected.

Tests

  • go test ./transport/openvpn ./adapter/outbound -count=1
  • go test -race ./transport/openvpn -count=1
  • go vet ./...
  • SKIP_INTEROP_TEST=1 SKIP_CONCURRENT_TEST=1 go test ./... -count=1
  • SKIP_INTEROP_TEST=1 SKIP_CONCURRENT_TEST=1 go test ./... -tags with_gvisor -count=1
  • Targeted deadline, Close, replay, parser, retransmission, and epoch-race regressions were stress-run repeatedly.

The branch is a single commit directly on the current upstream Alpha head.

@wwqgtxx

wwqgtxx commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

The PR does not fully implement the fault-tolerance semantics of the original tran-window. In the original implementation, if a new handshake fails, the old "lame-duck key" is retained and forwarding continues within the window; however, in the current code, if rekeying fails, failControl() is called, shutting down the entire client. That said, this "immediate shutdown upon rekey failure" behavior already existed in the PR base and is not a regression introduced by this specific PR. Perhaps we can revise it in a future PR.

@wwqgtxx

wwqgtxx commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

However, you need to fix the Go 1.20 compatibility issue; do not use context.AfterFunc directly—use our contextutils.AfterFunc instead.

@wwqgtxx

wwqgtxx commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Also need fix

=== RUN   TestControlWriteDeadlineExtensionIgnoresOldTimer
    control_test.go:1263: superseded deadline canceled write: context deadline exceeded
--- FAIL: TestControlWriteDeadlineExtensionIgnoresOldTimer (0.06s)

@ayanami-desu

ayanami-desu commented Aug 15, 2026

Copy link
Copy Markdown
Author

Verified and pushed as d332a31d. Resolution by item:

  1. AUTH_PENDING versus the 30-second rekey timeout: removed the fixed 30-second context deadline around the entire rekey. The 30-second deadline now only bounds initial TLS/KM2 progress. After receiving AUTH_PENDING,timeout N, the client updates the full ControlConn read/write deadline and honors the server-advertised deferred-authentication window. The retiring data key continues carrying traffic during its transition window.
  2. push-continuation versus the 30-second timeout: an incomplete continued PUSH during rekey now uses the retiring-key transition deadline instead of an unconditional 30-second deadline. If AUTH_PENDING and push-continuation are both active, the earlier of their two deadlines is used.
  3. Fail-closed policy: once a soft reset has advanced the reliable control epoch and started a new TLS byte stream, rollback is not safe. The client keeps waiting and using the retiring key while the advertised authentication, continuation, and transition deadlines remain valid. It fails closed only after those deadlines expire. This rationale is now documented next to the failure path.
  4. Real TLS rekey coverage: added an end-to-end test over the production ControlChannel/ControlConn using a real tls.Clienttls.Server exchange: soft reset → TLS handshake → KM2 → AUTH_PENDING → continued PUSH → new data epoch. It covers both plain and tls-auth control wrappers, and the deferred response intentionally arrives after the test's initial handshake timeout. I did not add a Docker/OpenVPN-binary interop harness because it would introduce an external runtime/network dependency into the test suite.
  5. Dead code/API: removed DataChannel.Started / sendStarted and their test; removed the unused ParsePushReplyFlexible, whose leftover return value was always nil.
  6. Data packet-ID exhaustion: the low-level data channel still rejects wraparound to prevent AEAD nonce reuse. The Client now logs once per epoch and drops packets instead of tearing down the tunnel; a subsequent server rekey installs a fresh counter and recovers automatically.
  7. Single parked soft-reset slot: documented the protocol invariant. A second distinct reset would require the peer to advance its key state again before the current epoch completes, so it is treated as a theoretical protocol violation; retransmissions of the same reset continue through the normal ACK path.
  8. Go 1.20 compatibility: replaced context.AfterFunc with the repository's common/contextutils.AfterFunc. GOTOOLCHAIN=go1.20.14 ... go test ./... passes.
  9. Windows flaky deadline test: replaced the scheduler-dependent 5 ms/20 ms timing test with a deterministic invocation of the generation-guarded stale timer callback.

Verification:

  • GOTOOLCHAIN=go1.20.14 SKIP_INTEROP_TEST=1 SKIP_CONCURRENT_TEST=1 go test ./... -count=1
  • go test -race ./transport/openvpn -count=1
  • go vet ./...
  • full default and with_gvisor repository test matrices
  • repeated stress runs for the real-TLS rekey and deadline regressions.

@wwqgtxx

wwqgtxx commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Do not turn data packet-ID exhaustion into silent success

transport/openvpn/client.go:765-769 catches errDataPacketIDExhausted, logs once, and returns nil without writing the packet. Nothing in the client initiates a local rekey, so recovery depends entirely on the peer eventually starting another epoch. With reneg-sec 0, or when a high packet rate reaches the limit before the peer's scheduled rekey, the tunnel remains reported as healthy while silently dropping every outbound packet, including keepalives, indefinitely.

OpenVPN does not wait for actual exhaustion: packet_id_close_to_wrapping() becomes true at 0xFF000000, and tls_process() starts a soft reset. The preferred fix is to trigger a client-side soft reset at that threshold. At minimum, propagate the exhaustion error so the adapter tears down and can reconnect instead of reporting successful delivery. TestClientDropsExhaustedDataPacketsUntilRekey currently codifies the silent-blackhole behavior and should be updated with the fix.

Anchor AUTH_PENDING after KM2 reaches the active key state

transport/openvpn/client.go:232 records controlEstablishedAt immediately after tls.Conn.HandshakeContext() returns, before doKeyExchange() has sent the client KM2 record or parsed the server KM2 record. Later, AUTH_PENDING,timeout N is calculated from that timestamp.

This is earlier than OpenVPN's key_state.established. In OpenVPN 2.6.22, session_move_active() sets ks->established only after key_method_2_read() has succeeded and the reliable send buffer has been acknowledged. Consequently, UDP loss or retransmission that delays KM2 shortens mihomo's deferred-authentication window by the KM2 delay. For example, if KM2 finishes 20 seconds after the TLS handshake and the server sends AUTH_PENDING,timeout 60, OpenVPN permits approximately 60 seconds from KM2 activation, while mihomo expires it after only approximately 40 more seconds.

Move the anchor to the point where the server KM2 record has been successfully parsed and the new key state is ready to become active. The existing TestAuthPendingDeadlineAnchoredAtTLSEstablishment verifies that an injected timestamp is preserved, but it does not verify that production captures that timestamp at the OpenVPN-equivalent state transition.

@ayanami-desu

Copy link
Copy Markdown
Author

Addressed in 3dec0b8f.

Data packet-ID exhaustion

The silent-success path has been removed. Client.writeDataPacket now propagates the encryption error unchanged, so both the adapter packet loop and keepalive loop terminate through their existing shared stop path. That closes the client/TUN, clears the running adapter state, and allows the next use to reconnect instead of leaving a healthy-looking blackhole.

I also moved the cutoff from actual uint32 exhaustion to OpenVPN's PACKET_ID_WRAP_TRIGGER (0xFF000000). Packet ID 0xFF000000 is the final emitted ID; every subsequent attempt returns errDataPacketIDExhausted without incrementing the counter or writing to the transport. This is the conservative reconnect fallback rather than implementing client-initiated soft reset in this PR.

Tests now verify:

  • the threshold and non-advancing failure for AES-GCM, AES-CBC, and ChaCha20-Poly1305;
  • propagation through Client.WriteIPPacket;
  • no packet reaches the transport after the threshold.

TestClientDropsExhaustedDataPacketsUntilRekey was replaced with TestClientPropagatesDataPacketIDExhaustion.

AUTH_PENDING establishment anchor

controlEstablishedAt is now cleared when a new TLS epoch starts and captured only after:

  1. the server KM2 record has been parsed successfully; and
  2. the new key material has been derived successfully.

This occurs before any trailing/coalesced AUTH_PENDING or PUSH data is consumed, so those messages use the OpenVPN-equivalent KM2 activation point rather than TLS handshake completion. Failed TLS, KM2 parsing, or key derivation cannot install a new anchor, and a previous epoch's timestamp cannot leak into the new epoch.

The real TLS rekey test now preloads a stale prior-epoch timestamp, completes TLS and client KM2, blocks the server before server KM2, and verifies that the anchor is still zero. It then releases server KM2 and verifies both the new anchor position and deferredUntil == establishedAt + timeout, for plain and tls-auth variants.

Verification after the final patch:

  • focused regressions repeated 10 times;
  • go test ./transport/openvpn ./adapter/outbound -count=1;
  • go test -race ./transport/openvpn -count=1;
  • go vet ./...;
  • GOTOOLCHAIN=go1.20.14 SKIP_INTEROP_TEST=1 SKIP_CONCURRENT_TEST=1 go test ./... -count=1;
  • full default and with_gvisor repository matrices.

I also ran three independent read-only subagent reviews covering packet-ID/security boundaries, AUTH_PENDING/KM2 lifecycle, and the complete diff/callsites. All three reported no actionable findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants